-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support "host" resources #62
Conversation
Closes APIARY-5854
6f683ac
to
184a297
Compare
I am not completely sure about this one... |
In the last commit |
element-definitions
element-definitions
docs/element-definitions.md
Outdated
@@ -1139,6 +1139,10 @@ The Resource representation with its available transitions and its data. | |||
|
|||
The `content` MUST NOT include more than one `Data Structure`. | |||
|
|||
#### Classifications | |||
|
|||
- `"hosts"` - Resource is a set of servers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be
"host"
- Resource is a host description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, explain what a "host" resource is in the description of the resource element (L1123).
Something like "A resource element classified as host
SHALL be interpreted as...".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if this has been implemented in the end as in Resource we have hosts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a couple of suggestions, one is regarding adding the hosts
attribute to both Resource and Transition. This is to support the OpenAPI feature of overriding the host at the resource or transition element.
The other is to include further semantics and explanation for the reader for the host
resource classification.
In terms of API Elements JS, we likely want to add the following:
hosts
property toCategory
which looks for hosts categories, something likeitem.classes.contains(‘hosts’)
.hosts
property onResource
which looks inthis.attributes
for thehosts
category.hosts
property onTransition
which looks inthis.attributes
for thehosts
category.
- for Resource and Transition - refactoring
e6be9ae
to
66dcf39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, I'll let @tjanc have another look before we merge otherwise looks great 👍.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some wording/fixes. @kylef feedback appreciated, my the english is not the perfectest.
Also, @marcofriso please add an entry to the overview here:
https://github.com/apiaryio/api-elements/blob/master/docs/overview.md#relationship-of-elements
Something like
Category (Group of Resource Elements representing hosts)
after
Category (Group of Resource Elements)
Category (Group of Authentication and Authorization Scheme Definitions)
docs/element-definitions.md
Outdated
@@ -1126,11 +1126,17 @@ The Resource representation with its available transitions and its data. | |||
|
|||
- `element` - `"resource"` | |||
- `attributes` | |||
- `hosts` ([Array][][[Resource][]]). | |||
|
|||
Resources in the hosts attribute SHOULD implicitly be treated as they have the `host` resource classification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps
Optional list of host resources. Every entry SHALL be interpreted as if classified as `host`.
_See `host` classification in [Resource][] for further semantics._
Overrides any otherwise relevant `hosts` definitions.
@kylef what do you think (wording)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the difference between SHALL and SHOULD in these circumstances?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SHALL describes a requirement. SHOULD describes a recommendation. See RFC 2119.
I think we should require any interpreter of APIE to see these entries as hosts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
docs/element-definitions.md
Outdated
|
||
Definition of any input message-body attribute for this transition. | ||
|
||
- `hosts` ([Array][][[Transition][]]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be
`hosts` ([Array][][[Resource][]])
Also, wording, similar to above comment:
Optional list of host resources. Every entry SHALL be interpreted as if classified as `host`.
_See `host` classification in [Resource][] for further semantics._
All [Resource][]s nested under the [Transition][]'s `content` SHALL interpret this `hosts` definition as their own, unless it is overridden by another `hosts` definition on the path to the [Resource][] element.
@kylef feedback appreciated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved 'See host
classification in [Resource][] for further semantics.' at the end of the paragraph.
docs/element-definitions.md
Outdated
- `"host"` - A host transition represents the "root" of the API transition. | ||
The transition href MAY be append to the host href to create a absolute URI. | ||
A transition that has a `host` classification MUST be a root component of a URI. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A transition cannot be a host no? Just a resource. I think we can remove this paragraph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being application state transitions ("Transitions") resource operations should we include here the Category of Hosts at all (#53 (comment))?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
docs/element-definitions.md
Outdated
- `"dataStructures"` - Category is a set of data structures. | ||
- `"hosts"` - Category of [Resource][] or [Transition][] which implicitly contain the respective `host` classification. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wording:
Category of [Resource][]s interpreted as a list of host resources of an API. Every entry SHALL be interpreted as if classified as `host`.
_See `host` classification in [Resource][] for further semantics._
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
I see also some of the internal links have to be changed as they don't work |
element-definitions
Update the specification at to include "Server Element" structure described by OAS 3 format (https://swagger.io/docs/specification/api-host-and-base-path/).
As reported on Swagger website:
In OAS 3.0 a simple server looks like this:
In Api Elements a server should contain a
href
and optionallyhrefVariables
.A simple OAS3 server would map to a server element like this:
For more information about the design please check the following link → #53 (comment)
UPDATE
→ after talk to @tjanc I am going to remove
server
object and introducehosts
classification under "Resource" and "Category"